Skip to content

Comments

Optimize/ebpf loading#718

Merged
matthyx merged 2 commits intomainfrom
optimize/ebpf-load
Feb 11, 2026
Merged

Optimize/ebpf loading#718
matthyx merged 2 commits intomainfrom
optimize/ebpf-load

Conversation

@matthyx
Copy link
Contributor

@matthyx matthyx commented Feb 10, 2026

Summary by CodeRabbit

Release Notes

  • New Features

    • Added tracer management system with coordinated initialization, staggered sequencing, and improved error handling during startup.
  • Chores

    • Host sensor is now disabled by default.

@matthyx matthyx changed the title Optimize/ebpf load Optimize/ebpf loading Feb 10, 2026
Signed-off-by: Matthias Bertschy <matthias.bertschy@gmail.com>
…ncurrent eBPF loading

Signed-off-by: Matthias Bertschy <matthias.bertschy@gmail.com>
@coderabbitai
Copy link

coderabbitai bot commented Feb 10, 2026

📝 Walkthrough

Walkthrough

The PR disables host sensor by default in configuration and introduces a TracerManager type to orchestrate tracer registration, retrieval, and lifecycle management with memory-conscious startup sequencing, error resilience, and procfs tracer initialization priority.

Changes

Cohort / File(s) Summary
Configuration
pkg/config/config.go
Changed default value of hostSensorEnabled from true to false.
Tracer Management
pkg/containerwatcher/v2/tracer_manager.go
Introduced TracerManager type with lifecycle management (RegisterTracer, GetTracer, StartAllTracers, StopAllTracers). Enhanced startup with memory-conscious sequencing (2-second delays between tracer starts), procfs-first initialization, and graceful per-tracer error handling without failing entire startup.

Sequence Diagram(s)

sequenceDiagram
    participant tm as TracerManager
    participant tf as TracerFactory
    participant procfs as Procfs Tracer
    participant other as Other Tracers
    participant ctx as Context

    tm->>tf: Initialize all tracers
    activate tf
    tf-->>tm: Return tracer instances
    deactivate tf
    
    tm->>procfs: Start procfs tracer first
    activate procfs
    procfs-->>tm: Startup complete
    deactivate procfs
    
    tm->>tm: Wait for procfs scan interval
    
    loop For each remaining tracer
        tm->>other: Start tracer
        activate other
        other-->>tm: Started (or error logged)
        deactivate other
        tm->>tm: Wait 2 seconds (memory-conscious delay)
    end
    
    note over tm: All tracers started with staggered timing<br/>to reduce kernel eBPF load contention
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~22 minutes

Poem

🐰 Hops of joy for tracers new,
A manager orchestrates the crew!
Procfs leads with careful grace,
Staggered starts set the pace,
And sensors sleep—defaults are true! ✨

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title is related to the main changes in the PR, which focus on optimizing eBPF loading through delayed startup and memory-conscious tracer management, though it is somewhat abbreviated/abbreviated.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch optimize/ebpf-load

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)
pkg/containerwatcher/v2/tracer_manager.go (2)

141-161: ⚠️ Potential issue | 🟠 Major

Unconditional 30s wait even when procfs tracer is absent or disabled.

The select block at lines 154–158 waits for ProcfsScanInterval (default 30s) regardless of whether the procfs tracer was found or started. If the procfs tracer doesn't exist in the map or is disabled, this is a pointless 30-second stall on every startup.

Move the wait inside the conditional block so it only applies when the tracer was actually started.

Proposed fix
 func (tm *TracerManager) startProcfsTracer(ctx context.Context) error {
 	if tracer, exists := tm.GetTracer(utils.ProcfsEventType); exists {
 		delete(tm.tracers, utils.ProcfsEventType)
 		if tracer.IsEnabled(tm.cfg) {
 			logger.L().Info("Starting procfs tracer before other tracers")
 			if err := tracer.Start(ctx); err != nil {
 				return fmt.Errorf("starting procfs tracer: %w", err)
 			}
+			// Wait for procfs scan to complete before starting eBPF tracers.
+			// This delay ensures container/process data is ready for enrichment.
+			select {
+			case <-time.After(tm.cfg.ProcfsScanInterval):
+			case <-ctx.Done():
+				return ctx.Err()
+			}
 		}
 	}
 
-	// Wait for procfs scan to complete before starting eBPF tracers.
-	// This delay ensures container/process data is ready for enrichment.
-	select {
-	case <-time.After(tm.cfg.ProcfsScanInterval):
-	case <-ctx.Done():
-		return ctx.Err()
-	}
-
 	return nil
 }

15-29: ⚠️ Potential issue | 🟡 Minor

thirdPartyTracers has no registration method and remains unused.

The thirdPartyTracers slice is initialized in the constructor but never populated. All tracers, including third-party tracers, are registered via RegisterTracer() which adds them to the tracers map. This makes the thirdPartyTracers field dead code, as stopThirdPartyTracers() iterates over an always-empty slice. Either remove this unused field or populate it during tracer registration if separate handling of third-party tracers is intended.

🧹 Nitpick comments (4)
pkg/containerwatcher/v2/tracer_manager.go (4)

76-118: Non-deterministic tracer startup order due to map iteration.

tm.tracers is a map[utils.EventType]TracerInterface, and Go map iteration order is randomized. This means the startup sequence of tracers (beyond the procfs tracer) will vary between runs. If the staggered startup is meant to optimize memory usage patterns, the lack of a stable order could make performance characteristics harder to reason about and reproduce.

Consider using an ordered data structure (e.g., a []TracerInterface slice) for startup ordering, or sort the map keys before iterating.


96-117: Unnecessary 2s delay after the last tracer starts.

The delay select is executed after every tracer start including the last one, adding an unnecessary 2-second wait at the end of StartAllTracers. You could skip the delay for the last tracer or move the delay to the beginning of each iteration (skip on first).


49-51: GetAllTracers exposes internal mutable state.

Returning the internal tm.tracers map directly allows callers to mutate it (add/delete entries). If this is intentional for the factory pattern, that's fine, but it's worth noting the coupling. A read-only copy or a dedicated registration callback would be safer.


123-133: StopAllTracers discards all errors except the last.

When multiple tracers fail to stop, only lastErr is returned. Consider using errors.Join (Go 1.20+) to aggregate all shutdown errors, making debugging easier when multiple tracers fail simultaneously.

Proposed fix
 func (tm *TracerManager) StopAllTracers() error {
 	tm.stopThirdPartyTracers()
 
-	var lastErr error
+	var errs []error
 	for _, tracer := range tm.tracers {
 		if err := tracer.Stop(); err != nil {
-			lastErr = err
+			errs = append(errs, err)
 		}
 	}
-	return lastErr
+	return errors.Join(errs...)
 }

@matthyx matthyx added the release Create release label Feb 10, 2026
@matthyx matthyx merged commit 5359542 into main Feb 11, 2026
27 checks passed
@matthyx matthyx deleted the optimize/ebpf-load branch February 11, 2026 20:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

release Create release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant